Conversation
Rockjack00
force-pushed
the
recursive_behavior_trees
branch
from
d17af90 to
0f26b68
Compare
|
thanks a lot. I will review it soon |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses Issue #979 by adding runtime checks to catch recursive behavior tree definitions and providing test cases to verify these conditions.
- Added two test cases (RecursiveSubtree and RecursiveCycle) to validate that cycles in behavior trees trigger errors.
- Introduced a recursion check in the XML parser to detect subtree cycles during parsing.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/gtest_subtree.cpp | Added tests that ensure recursive trees and cycles are rejected. |
| src/xml_parsing.cpp | Implemented a recursion check for subtree creation in the XML parser. |
| const std::string subtree_ID = element->Attribute("ID"); | ||
|
|
||
| // check for recursion in behavior tree | ||
| if(prefix.find(subtree_ID) != std::string::npos) |
There was a problem hiding this comment.
Consider using a delimiter-aware search to verify the presence of subtree_ID within prefix. This ensures that matching does not produce false positives if one subtree ID is a substring of another.
| if(prefix.find(subtree_ID) != std::string::npos) | |
| std::string subtree_delimited = subtree_ID + "/"; | |
| if(prefix == subtree_ID || prefix.find(subtree_delimited) == 0) |
There was a problem hiding this comment.
ensures that matching does not produce false positives if one subtree ID is a substring of another.
Good idea, but the provided solution does not achieve this because of the added ::<UID> for subtree paths.
Is there a method used elsewhere in the library for extracting tokens from a path that could be used for this? Otherwise I can add a string splitting routine. I'm not quite as C++ literate, but in python it would look something like this:
import re
ancestors = re.split(r'::[0-9]*/?', prefix)
if subtree_ID in ancestors:
raise RuntimeError( ... )
|
can you have a look at #1085 ? |
This PR addresses Issue #979. It checks for the existence of a subtree in its own prefix while parsing and includes two test cases for recursive trees mentioned in the issue.